Revamp checksum - retry will reuse the checksum#532
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #532 +/- ##
==========================================
+ Coverage 88.66% 88.75% +0.08%
==========================================
Files 21 22 +1
Lines 6652 6722 +70
==========================================
+ Hits 5898 5966 +68
- Misses 754 756 +2
🚀 New features to boost your workflow:
|
| #include <aws/common/byte_buf.h> | ||
| #include <aws/common/ref_count.h> | ||
|
|
||
| struct aws_s3_meta_request_checksum_config_storage; |
There was a problem hiding this comment.
does it need to be in a separate header?
with all the cross header forward declarations it seems the 2 headers are tightly coupled
There was a problem hiding this comment.
it doesn't need to be separate, but I'd think it be more clear to have them separate.
| struct aws_byte_buf *buffer, | ||
| uint32_t part_number, | ||
| const struct aws_string *upload_id, | ||
| bool should_compute_content_md5, |
There was a problem hiding this comment.
not for now, but we should really unify md5 with the rest of checksum at some point
|
should we sanity check that the size of data pushed through the checksum matches expected payload size? we can even check response size here? |
In the case of the checksum was calculated for some portion of the part, and client somehow reused the checksum, the server side will be able to catch the error since the checksum won't match the full part of data client sent. So, it's less a concern here. |
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.